Hi @zhouhao3. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

IMO adding Restart=on-failure is a code improvement, not a bug fix.

When testing IPI with #33 on iRMC, ironic-agent sometimes failed to start successfully, but after a manual restart, the service ran successfully. We noticed that the error was related to the network. Unfortunately, we didn't record the error message at that time. Now we've done 6 tests in a row but can't reproduce the error. We guess that when the service was pulling the image, the proxy was just unavailable, causing the service to fail to start.

Therefore, if the service itself supports restart after failure, it can avoid permanent failure of the service due to factors such as network instability.

/ok-to-test

The error reappeared.

-- Logs begin at Fri 2022-01-28 23:33:55 UTC, end at Fri 2022-01-28 23:38:23 UTC. --
Jan 28 23:34:41 master-1 systemd[1]: Starting Ironic Agent...
Jan 28 23:34:42 master-1 podman[2243]: Trying to pull quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:b9f235258d1bf30f5819089506e1ee5b698dcbe1b52ce9e49cbaad6ad96c1897...
Jan 28 23:34:46 master-1 podman[2243]: Getting image source signatures
Jan 28 23:34:50 master-1 podman[2243]: Error: Error reading blob sha256:eac1b95df832dc9f172fd1f07e7cb50c1929b118a4249ddd02c6318a677b506a: Get "https://cdn02.quay.io/sha256/ea/eac1b95df832dc9f172fd1f07e7cb50c1929b118a4249ddd02c6318a677b506a?Expires=1643413370&Signature=Tix43Y0YVkJaSgjvc6stWUiASYCRxEc7C-JSNge-R4zbwrS9bhgNNIMJDBpy4lbNj-r7KJJw7q~rmO8xiI47sYJkI1zLRAFFijTFDVCKbNGxaYUBfUvrC0BEcCdLTosgkS8m2Nv1BOqjQcZI61B2ChjLMtYFzmwllSOMga9UV9A2iZJDTi9pSjLPaFl-qam1W57FJQIh107gfA3lo7NounE337WKt6rQWSVgFfvFe0G~Sjs3yMknHcCvjuiKlF86F05RJAD0VBPMOCnDqYHLHOA~-6d-ZmSmBqK307Ab2~Bic~eqahQKxewWuRK70AWtG24LLKlDeGBumH006nTKSw__&Key-Pair-Id=APKAJ67PQLWGCSP66DGA": ERROR
Jan 28 23:34:50 master-1 systemd[1]: ironic-agent.service: Control process exited, code=exited status=125
Jan 28 23:34:50 master-1 systemd[1]: ironic-agent.service: Failed with result 'exit-code'.
Jan 28 23:34:50 master-1 systemd[1]: Failed to start Ironic Agent.

It happened again:

-- Logs begin at Sat 2022-01-29 06:31:32 UTC, end at Sat 2022-01-29 07:46:48 UTC. --
Jan 29 06:32:19 worker-0 systemd[1]: Starting Ironic Agent...
Jan 29 06:32:20 worker-0 podman[2277]: Trying to pull quay.io/openshift-release-dev/ocp-v4.0-art-dev@sha256:f07e1596763e4f1b0834cb700037fd1958071ae26492d8c3fc686afc5ef560cb...
Jan 29 06:32:26 worker-0 podman[2277]: Error: Error parsing image configuration: Get "https://cdn02.quay.io/sha256/db/db1135106ca881540040a27ecd1e8d3af97eff3881095ea9a48a1f5b1d1e407d?Expires=1643438266&Signature=NFPoGINK-IW0lgieHhLa~G-9m462emXWjEvcXqMJhMAJSNPZWrA4ywibhgxguUYy60GZFRVpZcZsm0OQ3aRhLkbTrRPCgv6H3ndmCV-kpHgvqajCm-QZM5PBI~pdnNajxyLPKqfVEZq9cntOE3CidbERlUNT~92P4cL2Zlp~nE1JHvFS~Y5BkpvscQWshEUEBlc~ZmAPO1QF9CSGbLlB7TFsNg22M65rpikNqHszyk9N5STuq6p5SqrRTGs78JUm-AIEDq~CZWfJVcHTqliCUxfMvSJEsl-wLIfcz2NfQIhctDHwrmGVEWdKjMs1Y7T4UkHGb5IEpBoarp~nApvSkw__&Key-Pair-Id=APKAJ67PQLWGCSP66DGA": ERROR
Jan 29 06:32:26 worker-0 systemd[1]: ironic-agent.service: Control process exited, code=exited status=125
Jan 29 06:32:26 worker-0 systemd[1]: ironic-agent.service: Failed with result 'exit-code'.
Jan 29 06:32:26 worker-0 systemd[1]: Failed to start Ironic Agent.

In case of non-transient failure, with this approach isn't the service restarted continuosly?

Restarting the service matches the behavior of our previous ramdisk, so it should be fine.

Please fix the unit test failure.

Restarting the service matches the behavior of our previous ramdisk, so it should be fine.

Yes, I was thinking more to the fact that in case of permanent failure (ie network outage) the installation will probably fail after the bootstrap timeout (30m), while a mechanism based on a counter could fail faster.

I don't think we have a way to communicate the agent failure back to the control plane.

I don't think we have a way to communicate the agent failure back to the control plane.

Right, not sure if at least the error reporting could be improved then ( I guess @hs0210 fetched the journal log directly from within the instance?)

/retest

@dtantsur @andfasano updated, PTAL.

/retest

@zhouhao3: all tests passed!

Full PR test history. Your PR dashboard.

Right, not sure if at least the error reporting could be improved then ( I guess @hs0210 fetched the journal log directly from within the instance?)

We can log in to the master to get the journal log, but we haven't thought of a good way to improve the error report.

@dtantsur @andfasano When we use IPI deployment, we still encounter this problem from time to time. Can you review this PR? thanks.

/lgtm

/approve

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andfasano, zhouhao3

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Can we port it to 4.10 as well?

/cherry-pick release-4.10

@elfosardo: new pull request created: #52